fix(s3-compat): v0.33.0 — provider-compatibility correctness + frontend desync + injection defense-in-depth#119
Conversation
…nd desync + injection defense-in-depth The core promise (works against AWS AND MinIO/Ceph/R2/B2) got its first dedicated correctness pass, plus two frontend desync fixes and one prompt-injection defense-in-depth line. Found by a fresh three-angle audit. S3 correctness & provider compatibility: - review_bucket_security: no bucket policy (NoSuchBucketPolicy) now reads as policy-not-public instead of "unknown", matching the survey path — a clean bucket is no longer reported as "cannot rule out public". - test_addressing_style on an IP endpoint (MinIO/Ceph) no longer falsely reports both_work: botocore never virtual-hosts an IP, so it detects the IP and reports path-style with a note. - list_object_versions/list_multipart_uploads map 501/NotImplemented (and a bare 405) to provider_unsupported, not a hard failure read as "0 versions" (rule 18). - Paging made real: the agent wrappers thread key/version/upload/part markers the S3 layer already accepted (first-page counts were reported as totals); list_upload_parts accepts PartNumberMarker; list_buckets follows ContinuationToken with a page cap + list_truncated. - evidence _list_prefix returns a truncation flag (surfaced as a plan warning) + a page-budget guard against empty-token infinite loops; a 5000-key silent cap had dropped the newest chronologically-sorted logs. - region_mismatch skips a custom endpoint whose bucket returns an empty LocationConstraint (no false "wrong signing region" on MinIO/Ceph). Frontend: - A transient reload failure at turn completion no longer erases the streamed answer: reload reports success and the bubble is kept (stalled) on a blip. - Concurrent same-session reloads guarded by a monotonic token (stale can't clobber fresh). - Health poll clears its abort timer on fetch rejection too. Security (defense-in-depth): - A safety rule tells the agent that tool-result text (bucket/object names, previewed bodies, config rules, logs) is untrusted data, never instructions. Structurally sound already (untrusted data only reaches the model via the tool channel; all tools read-only; EXPENSIVE ops confirmation-gated) — ceiling is a wasted turn, not RCE. Tests: +11 (test_v0330_fixes.py) + updated the rule-18 versions test. Full sidecar suite 584 passed; ruff clean; tsc + frontend build clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3YojFPzjkfMj6YYSwvzdx
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 131f2e90ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cont = None | ||
| truncated = False | ||
| for page in range(_MAX_BUCKET_PAGES): | ||
| resp = client.list_buckets(**({"ContinuationToken": cont} if cont else {})) |
There was a problem hiding this comment.
When an AWS account has an approved bucket quota above 10,000, the ListBuckets API rejects unpaginated requests; the AWS docs' paginated examples use max-buckets on the initial request. This loop sends no parameters while cont is null, so the first call is still unpaginated and fails before any ContinuationToken can be followed, leaving large accounts unable to list buckets despite the new pagination path. Pass MaxBuckets on every page.
Useful? React with 👍 / 👎.
| // still lacks the new messages, so clearing the streamed bubble now | ||
| // would erase the answer the user just watched complete. Keep it and | ||
| // mark stalled so they get a reload affordance instead (F1). | ||
| patchSessionRun(id, { stalled: true }); |
There was a problem hiding this comment.
When a successful streamed turn hits a transient reload failure, streamText/streamTools are still populated. Thread.tsx only renders the stalled reload card in the branch where both are empty; with streamed content it stays in the live message branch with streaming={!run.stopped} and no reload button, so the finished answer appears to stream forever. Clear the stream state here or make the render branch honor run.stalled first.
Useful? React with 👍 / 👎.
2 participants
What changed
The product's core promise — works against AWS and MinIO/Ceph/R2/B2 — got its first dedicated correctness pass, plus two frontend desync fixes and one prompt-injection defense-in-depth line. Found by a fresh three-angle audit (untrusted-data ingestion, S3-compat edge cases, frontend runtime).
🔴 S3 correctness (HIGH)
review_bucket_securityset the policy verdict only whenGetBucketPolicyStatusreturned a value; the common case (no policy →NoSuchBucketPolicy) left the combined verdict at "cannot rule out public" even with a clean, readable ACL — and disagreed with the survey path, which already maps it. Absent ≠ unknown.both_workon the most common S3-compatible setup. Now detects the IP and reports path-style with a note.🟠 S3 correctness (MEDIUM)
list_object_versions/list_multipart_uploadsmappedNotImplemented/501(and now a bare405) to a hard failure the agent read as "0 versions / 0 uploads". Nowprovider_unsupported, like the sibling object tools.list_upload_partsacceptsPartNumberMarker;list_bucketsfollowsContinuationToken(page cap +list_truncated)._list_prefixstopped at 5000 objects with no signal; since log/inventory keys sort chronologically, a recent-window query silently missed the newest logs. Now returns a truncation flag (→ plan warning) + a page-budget guard against empty-token infinite loops.LocationConstraintwas normalized tous-east-1then flagged mismatched against any region label. The check now skips a custom endpoint whose bucket reports no region.🟠 Frontend
reloadnow reports success; the bubble is kept (marked stalled, with a reload affordance) on a blip.🟢 Security (defense-in-depth)
What checks were run
cd sidecar && pytest -q— 584 passed (+11 intests/test_v0330_fixes.py; updated the rule-18 versions test).ruff check app— clean.cd frontend && npm run typecheck && npm run build— clean.What was not run
Known gaps / deferred
setTimeoutcleanup; a one-frame duplicate-bubble flash) deferred as non-bugs: in React 18 asetStateafter unmount is a silent no-op, and the flash "almost certainly won't paint."Security invariants
No new writes, no shell, no destructive S3 op. All new/changed S3 tools stay read-only and bounded;
provider_unsupportedmapping widened (rule 18), never narrowed. Bucket/object names still bypass redaction deliberately (they're reused asBucket=args) — secrets in error messages/headers are still redacted.🤖 Generated with Claude Code
https://claude.ai/code/session_01M3YojFPzjkfMj6YYSwvzdx
Generated by Claude Code